Skip to content

Conversation

itamar-starkware
Copy link
Contributor

@itamar-starkware itamar-starkware commented Oct 5, 2025

Change metric recorder to be global.
In the next PR we wil use different tokio runtime across different OS threads so we need a global metric handler for aligned results.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Contributor Author

itamar-starkware commented Oct 5, 2025

@itamar-starkware itamar-starkware force-pushed the 10-05-apollo_integration_tests_make_metric_recorder_init_global branch from b87fe2b to 9a1238c Compare October 5, 2025 09:10
@itamar-starkware itamar-starkware self-assigned this Oct 5, 2025
@itamar-starkware itamar-starkware marked this pull request as ready for review October 5, 2025 09:12
@itamar-starkware itamar-starkware changed the title apollo_integration_tests: make metric recorder init global in flow tests apollo_integration_tests: make metrics recorder init global in flow tests Oct 5, 2025
Copy link
Contributor

@ArniStarkware ArniStarkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @alonh5)


crates/apollo_integration_tests/tests/common/mod.rs line 140 at r1 (raw file):

    let metrics = handle.render();
    apollo_batcher::metrics::BATCHED_TRANSACTIONS.parse_numeric_metric::<usize>(&metrics).unwrap()
}

This function specifically expects a prometheusHandle of a recorder, correct?

Also - be consistent with assert_full_blocks_flow

Suggestion:

fn get_total_batched_txs_count(recorder_handle: &PrometheusHandle) -> usize {
    let metrics = recorder_handle.render();
    apollo_batcher::metrics::BATCHED_TRANSACTIONS.parse_numeric_metric::<usize>(&metrics).unwrap()
}

@itamar-starkware itamar-starkware force-pushed the 10-05-apollo_integration_tests_make_metric_recorder_init_global branch from 9a1238c to 7ee1793 Compare October 9, 2025 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants